tests: Require `run-fail` ui tests to have an exit code (`SIGABRT` not ok) #143002

Enselic · 2025-06-25T06:10:21Z

And introduce two new directives for ui tests:

run-crash
run-fail-or-crash

Normally a run-fail ui test like tests that panic shall not be terminated by a signal like SIGABRT. So begin having that as a hard requirement.

Some of our current tests do terminate by a signal/crash however. Introduce and use run-crash for those tests. Note that Windows crashes are not handled by signals but by certain high bits set on the process exit code. Example exit code for crash on Windows: 0xc000001d (STATUS_ILLEGAL_INSTRUCTION). Because of this, we define "crash" on all platforms as "not exit with success and not exit with a regular failure code in the range 1..=127".

Some tests behave differently on different targets:

Targets without unwind support will abort (crash) instead of exit with failure code 101 after panicking. As a special case, allow crashes for run-fail tests for such targets.
Different sanitizer implementations handle detected memory problems differently. Some abort (crash) the process while others exit with failure code 1. Introduce and use run-fail-or-crash for such tests.

This adds further (cc #142304, #142886) protection against the regression in #123733 since that bug also manifested as SIGABRT in tests/ui/panics/panic-main.rs (shown as Aborted (core dumped) in the logs attached to that issue, and I have also been able to reproduce this locally).

TODO

Q: what about on Windows?
A: we'll treat any exit code outside of 1 - 127 as "crashed", and we'll do the same on unix.
test all permutations of actual vs expected
Done: See tests: Require run-fail ui tests to have an exit code (SIGABRT not ok) #143002 (comment).
Handle targets without unwind support
Add run-fail-or-crash for some sanitizer tests
remote-test-client

Zulip discussion

See https://rust-lang.zulipchat.com/#narrow/channel/122651-general/topic/compiletest.3A.20terminate.20by.20signal.20vs.20exit.20with.20error/with/525611235

// try-job: aarch64-apple
// try-job: x86_64-msvc-1
// try-job: x86_64-gnu
// try-job: dist-i586-gnu-i586-i686-musl
// try-job: test-various
try-job: armhf-gnu

rustbot · 2025-06-25T06:10:25Z

r? @petrochenkov

rustbot has assigned @petrochenkov.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

rustbot · 2025-06-25T06:10:29Z

Some changes occurred in src/tools/compiletest

cc @jieyouxu

workingjubilee · 2025-06-25T08:18:58Z

why not simply run-abort

also would accept run-crash

jieyouxu · 2025-06-25T10:10:37Z

@bors2 delegate=try

rust-bors · 2025-06-25T10:10:40Z

@Enselic can now perform try builds on this pull request

Enselic · 2025-06-25T11:37:25Z

@bors2 try jobs=x86_64-msvc-1,x86_64-msvc-2

rust-bors · 2025-06-25T11:37:29Z

⌛ Trying commit 17be091 with merge 873ecba…

To cancel the try build, run the command @bors2 try cancel.

…try> tests: Require `run-fail` ui tests to have an exit code (`SIGABRT` not ok) Normally a `run-fail` ui test shall not be terminated by a signal like `SIGABRT`. So begin having that as a hard requirement. Some of our current tests do terminate by a signal however. Introduce and use `run-fail-without-exit-code` for those tests. This adds further (cc #142304, #142886) protection against the regression in #123733 since that bug also manifested as `SIGABRT` in `tests/ui/panics/panic-main.rs` (shown as `Aborted (core dumped)` in the logs attached to that issue, and I have also been able to reproduce this locally). ### TODO - [ ] what about on Windows? - [ ] also update docs at https://rustc-dev-guide.rust-lang.org/tests/directives.html#controlling-outcome-expectations - [ ] clean up the code ### Zulip discussion See https://rust-lang.zulipchat.com/#narrow/channel/122651-general/topic/compiletest.3A.20terminate.20by.20signal.20vs.20exit.20with.20error/with/525611235 try-job: x86_64-msvc-1 try-job: x86_64-msvc-2

Enselic · 2025-06-25T13:34:03Z

An only-windows test that was run-fail now must be run-crash. Fixed now. Trying again:

@bors2 try jobs=x86_64-msvc-1,x86_64-msvc-2

rust-bors · 2025-06-25T13:34:07Z

⌛ Trying commit ad4e082 with merge e685982…

To cancel the try build, run the command @bors2 try cancel.

…try> tests: Require `run-fail` ui tests to have an exit code (`SIGABRT` not ok) Normally a `run-fail` ui test shall not be terminated by a signal like `SIGABRT`. So begin having that as a hard requirement. Some of our current tests do terminate by a signal however. Introduce and use `run-fail-without-exit-code` for those tests. This adds further (cc #142304, #142886) protection against the regression in #123733 since that bug also manifested as `SIGABRT` in `tests/ui/panics/panic-main.rs` (shown as `Aborted (core dumped)` in the logs attached to that issue, and I have also been able to reproduce this locally). ### TODO - [ ] **Q:** what about on Windows? **A:** we'll treat any exit code outside of 1 - 127 as "crashed", and we'll do the same on unix. - [ ] also update docs at https://rustc-dev-guide.rust-lang.org/tests/directives.html#controlling-outcome-expectations - [ ] clean up the code - [ ] test all permutations of actual vs expected ### Zulip discussion See https://rust-lang.zulipchat.com/#narrow/channel/122651-general/topic/compiletest.3A.20terminate.20by.20signal.20vs.20exit.20with.20error/with/525611235 try-job: x86_64-msvc-1 try-job: x86_64-msvc-2

src/tools/compiletest/src/runtest.rs

petrochenkov · 2025-07-03T13:13:55Z

r=me with #143002 (comment) addressed.
@rustbot author
@bors delegate+

bors · 2025-07-03T13:13:58Z

✌️ @Enselic, you can now approve this pull request!

If @petrochenkov told you to "r=me" after making some further change, please make that change, then do @bors r=@petrochenkov

…t ok) And introduce two new directives for ui tests: * `run-crash` * `run-fail-or-crash` Normally a `run-fail` ui test like tests that panic shall not be terminated by a signal like `SIGABRT`. So begin having that as a hard requirement. Some of our current tests do terminate by a signal/crash however. Introduce and use `run-crash` for those tests. Note that Windows crashes are not handled by signals but by certain high bits set on the process exit code. Example exit code for crash on Windows: `0xc000001d`. Because of this, we define "crash" on all platforms as "not exit with success and not exit with a regular failure code in the range 1..=127". Some tests behave differently on different targets: * Targets without unwind support will abort (crash) instead of exit with failure code 101 after panicking. As a special case, allow crashes for `run-fail` tests for such targets. * Different sanitizer implementations handle detected memory problems differently. Some abort (crash) the process while others exit with failure code 1. Introduce and use `run-fail-or-crash` for such tests.

Enselic · 2025-07-03T16:49:57Z

@bors r=@petrochenkov

bors · 2025-07-03T16:49:59Z

📌 Commit 6950daf has been approved by petrochenkov

It is now in the queue for this repository.

bors · 2025-07-04T03:19:45Z

⌛ Testing commit 6950daf with merge f6080af...

…etrochenkov tests: Require `run-fail` ui tests to have an exit code (`SIGABRT` not ok) And introduce two new directives for ui tests: * `run-crash` * `run-fail-or-crash` Normally a `run-fail` ui test like tests that panic shall not be terminated by a signal like `SIGABRT`. So begin having that as a hard requirement. Some of our current tests do terminate by a signal/crash however. Introduce and use `run-crash` for those tests. Note that Windows crashes are not handled by signals but by certain high bits set on the process exit code. Example exit code for crash on Windows: `0xc000001d` (`STATUS_ILLEGAL_INSTRUCTION`). Because of this, we define "crash" on all platforms as "not exit with success and not exit with a regular failure code in the range 1..=127". Some tests behave differently on different targets: * Targets without unwind support will abort (crash) instead of exit with failure code 101 after panicking. As a special case, allow crashes for `run-fail` tests for such targets. * Different sanitizer implementations handle detected memory problems differently. Some abort (crash) the process while others exit with failure code 1. Introduce and use `run-fail-or-crash` for such tests. This adds further (cc #142304, #142886) protection against the regression in #123733 since that bug also manifested as `SIGABRT` in `tests/ui/panics/panic-main.rs` (shown as `Aborted (core dumped)` in the logs attached to that issue, and I have also been able to reproduce this locally). ### TODO - [x] **Q:** what about on Windows? **A:** we'll treat any exit code outside of 1 - 127 as "crashed", and we'll do the same on unix. - [x] test all permutations of actual vs expected **Done:** See #143002 (comment). - [x] Handle targets without unwind support - [x] Add `run-fail-or-crash` for some sanitizer tests ### Zulip discussion See https://rust-lang.zulipchat.com/#narrow/channel/122651-general/topic/compiletest.3A.20terminate.20by.20signal.20vs.20exit.20with.20error/with/525611235 try-job: aarch64-apple try-job: x86_64-msvc-1 try-job: x86_64-gnu try-job: dist-i586-gnu-i586-i686-musl try-job: test-various

bors · 2025-07-04T04:20:07Z

💔 Test failed - checks-actions

Enselic · 2025-07-04T06:29:34Z

Looks like remote-test-client exits with code 3 if the remote crashes with SIGABRT. Will investigate more deeply when I get time. Probably makes sense to special-case this in compiletest.

@rustbot author

Enselic · 2025-07-04T16:58:04Z

@bors2 try jobs=armhf-gnu

rust-bors · 2025-07-04T16:58:08Z

⌛ Trying commit b5956bf with merge 84b0c2f…

To cancel the try build, run the command @bors2 try cancel.

…try> tests: Require `run-fail` ui tests to have an exit code (`SIGABRT` not ok) And introduce two new directives for ui tests: * `run-crash` * `run-fail-or-crash` Normally a `run-fail` ui test like tests that panic shall not be terminated by a signal like `SIGABRT`. So begin having that as a hard requirement. Some of our current tests do terminate by a signal/crash however. Introduce and use `run-crash` for those tests. Note that Windows crashes are not handled by signals but by certain high bits set on the process exit code. Example exit code for crash on Windows: `0xc000001d` (`STATUS_ILLEGAL_INSTRUCTION`). Because of this, we define "crash" on all platforms as "not exit with success and not exit with a regular failure code in the range 1..=127". Some tests behave differently on different targets: * Targets without unwind support will abort (crash) instead of exit with failure code 101 after panicking. As a special case, allow crashes for `run-fail` tests for such targets. * Different sanitizer implementations handle detected memory problems differently. Some abort (crash) the process while others exit with failure code 1. Introduce and use `run-fail-or-crash` for such tests. This adds further (cc #142304, #142886) protection against the regression in #123733 since that bug also manifested as `SIGABRT` in `tests/ui/panics/panic-main.rs` (shown as `Aborted (core dumped)` in the logs attached to that issue, and I have also been able to reproduce this locally). ### TODO - [x] **Q:** what about on Windows? **A:** we'll treat any exit code outside of 1 - 127 as "crashed", and we'll do the same on unix. - [x] test all permutations of actual vs expected **Done:** See #143002 (comment). - [x] Handle targets without unwind support - [x] Add `run-fail-or-crash` for some sanitizer tests ### Zulip discussion See https://rust-lang.zulipchat.com/#narrow/channel/122651-general/topic/compiletest.3A.20terminate.20by.20signal.20vs.20exit.20with.20error/with/525611235 // try-job: aarch64-apple // try-job: x86_64-msvc-1 // try-job: x86_64-gnu // try-job: dist-i586-gnu-i586-i686-musl // try-job: test-various try-job: armhf-gnu try-job: armhf-gnu

rust-bors · 2025-07-04T18:28:17Z

☀️ Try build successful (CI)
Build commit: 84b0c2f (84b0c2f8819b61873612b46e7192610acdf8e760, parent: 556d20a834126d2d0ac20743b9792b8474d6d03c)

Enselic · 2025-07-04T18:52:58Z

I think the most elegant solution to the remote-test-client problem is to make remote-test-client exit with code 128 + <signal-number> instead of always 3. The just finished try-job confirmed that it works. That way we don't need any special case in compiletest since it then follows the already established rules of:

run-fail code = 1..=127
run-crash code is outside of 0..=127 (or completely absent).

I opened a separate PR for that so we can land that beforehand and in isolation since the impact might be high (but maybe it isn't, not sure. If all of CI passes it's fine I guess): #143448

I opted for a random reviewer on that PR but maybe someone from here wants to approve that PR.

rustbot assigned petrochenkov Jun 25, 2025

Enselic added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 25, 2025

Enselic mentioned this pull request Jun 25, 2025

RUST_BACKTRACE=full loop with -Cpanic=abort on aarch64-unknown-linux-gnu #123733

Open

rustbot added the T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. label Jun 25, 2025

This comment has been minimized.

Sign in to view

Enselic force-pushed the tests-ui-run-fail-exit-vs-signal branch from 6ad1973 to 17be091 Compare June 25, 2025 10:12

This comment was marked as outdated.

Sign in to view

This comment was marked as resolved.

Sign in to view

Enselic force-pushed the tests-ui-run-fail-exit-vs-signal branch from 17be091 to ad4e082 Compare June 25, 2025 13:30

This comment was marked as resolved.

Sign in to view

Enselic force-pushed the tests-ui-run-fail-exit-vs-signal branch from ad4e082 to 2499dac Compare June 25, 2025 16:29

rustbot added the A-rustc-dev-guide Area: rustc-dev-guide label Jun 25, 2025

Enselic force-pushed the tests-ui-run-fail-exit-vs-signal branch from 7914e18 to 5334e8a Compare July 3, 2025 08:04

This comment has been minimized.

Sign in to view

Enselic force-pushed the tests-ui-run-fail-exit-vs-signal branch from 5334e8a to 7f2f79f Compare July 3, 2025 09:21

petrochenkov reviewed Jul 3, 2025

View reviewed changes

src/tools/compiletest/src/runtest.rs Outdated Show resolved Hide resolved

rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 3, 2025

rustbot added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jul 3, 2025

Enselic force-pushed the tests-ui-run-fail-exit-vs-signal branch from 7f2f79f to 6950daf Compare July 3, 2025 13:35

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 3, 2025

This comment has been minimized.

Sign in to view

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 4, 2025

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 4, 2025

TEST: remote-test-client: 128 + signal

b5956bf

Enselic mentioned this pull request Jul 4, 2025

remote-test-client: Exit code 128 + <signal-number> instead of 3 #143448

Open

tests: Require run-fail ui tests to have an exit code (SIGABRT not ok) #143002

Are you sure you want to change the base?

tests: Require run-fail ui tests to have an exit code (SIGABRT not ok) #143002

Conversation

Enselic commented Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

TODO

Zulip discussion

Uh oh!

rustbot commented Jun 25, 2025

Uh oh!

rustbot commented Jun 25, 2025

Uh oh!

This comment has been minimized.

workingjubilee commented Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jieyouxu commented Jun 25, 2025

Uh oh!

rust-bors bot commented Jun 25, 2025

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

Enselic commented Jun 25, 2025

Uh oh!

rust-bors bot commented Jun 25, 2025

Uh oh!

This comment was marked as resolved.

Enselic commented Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rust-bors bot commented Jun 25, 2025

Uh oh!

This comment was marked as resolved.

This comment has been minimized.

Uh oh!

petrochenkov commented Jul 3, 2025

Uh oh!

bors commented Jul 3, 2025

Uh oh!

Enselic commented Jul 3, 2025

Uh oh!

bors commented Jul 3, 2025

Uh oh!

bors commented Jul 4, 2025

Uh oh!

This comment has been minimized.

bors commented Jul 4, 2025

Uh oh!

Enselic commented Jul 4, 2025

Uh oh!

Enselic commented Jul 4, 2025

Uh oh!

rust-bors bot commented Jul 4, 2025

Uh oh!

rust-bors bot commented Jul 4, 2025

Uh oh!

Enselic commented Jul 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

tests: Require `run-fail` ui tests to have an exit code (`SIGABRT` not ok) #143002

tests: Require `run-fail` ui tests to have an exit code (`SIGABRT` not ok) #143002

Enselic commented Jun 25, 2025 •

edited

Loading

workingjubilee commented Jun 25, 2025 •

edited

Loading

Enselic commented Jun 25, 2025 •

edited

Loading

Enselic commented Jul 4, 2025 •

edited

Loading